Add str pointer methods #85816

WaffleLapkin · 2021-05-29T21:59:10Z

This PR adds the following functions for pointers to str, similar to already existing functions for pointers to [T].

impl *const str {
    pub const fn len(self) -> usize;
    pub const fn as_ptr(self) -> *const u8;
    pub unsafe fn get_unchecked<I>(self, index: I) -> *const I::Output
    where
        I: SliceIndex<str>;
}

impl *mut str {
    pub const fn len(self) -> usize;
    pub const fn as_mut_ptr(self) -> *mut u8;
    pub unsafe fn get_unchecked_mut<I>(self, index: I) -> *mut I::Output
    where
        I: SliceIndex<str>;
}

// mod ptr
pub const fn str_from_raw_parts(data: *const u8, len: usize) -> *const str;
pub const fn str_from_raw_parts_mut(data: *mut u8, len: usize) -> *mut str;

impl NonNull<str> {
    pub const fn str_from_raw_parts(data: NonNull<u8>, len: usize) -> Self;
    pub const fn len(self) -> usize;
    pub const fn as_non_null_ptr(self) -> NonNull<u8>;
    pub const fn as_mut_ptr(self) -> *mut u8;
    pub unsafe fn get_unchecked_mut<I>(self, index: I) -> NonNull<I::Output>
    where
        I: SliceIndex<str>;
}

I wasn't sure about feature gates and how to group them, so the advice is very appreciated.

This PR also adds const_str_ptr and mut_str_ptr lang items (they are required for *const str and *mut str impls).

See also:

len for raw slices -- Tracking Issue for raw slice len() method (slice_ptr_len, const_slice_ptr_len) #71146
as{,_mut,_non_null}_ptr, get_unchecked[_mut] -- Tracking Issue for raw slice getters (slice_ptr_get) #74265
ptr::slice_from_raw_parts[_mut] -- Tracking issue for ptr::slice_from_raw_parts[_mut] #36925

rust-highfive · 2021-05-29T21:59:14Z

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

jfrimmel · 2021-05-30T09:18:49Z

I'm unsure about ptr.as_ptr(), as we already have a pointer. Maybe use as_bytes() instead?

WaffleLapkin · 2021-05-30T10:12:40Z

@jfager I would expect that as_bytes returns some sort of byte slice, e.g. it's *const str -> *const [u8], I don't think it's a good name for a *const str -> *const u8 function. Maybe as_ptr isn't a great name either, but it's consistent with <*const [T]>::as_ptr.

jfager · 2021-05-30T18:26:17Z

@WaffleLapkin looks like you tagged the wrong person!

WaffleLapkin · 2021-05-30T19:07:50Z

Oops, right. I've meant to tag @jfrimmel 😅

m-ou-se · 2021-05-31T10:33:27Z

The len functions look fine to me. For those, you can just use the same tracking issue as for [T]: #71146

The as_[mut_]ptr and get_unchecked ones also look fine. Feel free to just re-use #74265.

But I'm not sure about the str_from_raw_parts, because we don't have a &str variant (yet?). Right now, you'd have to use slice::from_raw_parts and str::from_utf8_unchecked to make a &str from raw parts. If we have a str_from_raw_parts for *const str, I'm worried that might be confusing, since users searching for a &str will only find the *const str version. Is this ptr::str_from_raw_parts that useful? Should we leave it out? Or maybe we need one for &str too? (Part of the reason it's good it needs two steps, is that it makes it clear there's two unsafe assumptions: One about the pointer+len being valid, and one about the UTF-8 encoding being valid.)

WaffleLapkin · 2021-05-31T20:01:07Z

@m-ou-se my original motivation for ptr::str_from_raw_parts was that it seemed weird to have methods on *const str without a convenient way to create such pointers. &str on the other hand can be easily created, e.g. str::from_utf8(slice::from_raw_parts(ptr, len))?. Though for symmetry and convenience str::from_raw_parts seems nice.

Part of the reason it's good it needs two steps, is that it makes it clear there's two unsafe assumptions: One about the pointer+len being valid, and one about the UTF-8 encoding being valid.

Then maybe instead of ptr::str_from_raw_parts we could have str::from_raw_utf8(*const [u8]) -> *const str.

Not sure which way is better.

crlf0710 · 2021-07-04T04:35:09Z

@WaffleLapkin Ping from triage, what's next steps here?

m-ou-se · 2021-07-05T11:17:56Z

Then maybe instead of ptr::str_from_raw_parts we could have str::from_raw_utf8(*const [u8]) -> *const str.

Yeah maybe. I'd still put that in ptr:: instead of str:: though. Most users of str aren't interested in raw pointers, so it's probably good to keep all the raw pointer functions inside the ptr module.

cc @rust-lang/libs-api Any thoughts on these functions?

dtolnay · 2021-07-05T15:17:19Z

I don't see why the safety requirement of get_unchecked involves utf8 boundaries. I would expect only the same safety requirements as https://doc.rust-lang.org/std/primitive.pointer.html#method.add.

Please document safety requirements in a # Safety section.

Can we have safe indexing that works like https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_add ?

WaffleLapkin · 2021-07-05T18:14:40Z

I don't see why the safety requirement of get_unchecked involves utf8 boundaries. I would expect only the same safety requirements as https://doc.rust-lang.org/std/primitive.pointer.html#method.add.

Hm. So *const str doesn't have utf8 guarantees, and it's only unsafe to create non-utf8 &str?

WaffleLapkin · 2021-07-17T10:26:29Z

So, to summarize the state of this PR.

len, as_ptr, as_mut_ptr, as_non_null_ptr and get_unchecked are considered ok, I just need to fill tracking issues.
- I also need to fix safety comments (add # Safety, remove mentions of utf8 boundaries)
- There is an open question if get_unchecked could be safe if it used wrapping_add, but it equally applies to <[T]>::get_unchecked so IMO it's out of scope of this PR.
str_from_raw_parts{_mut} is more controversial, since it may be confusing to have a function *const u8, usize -> *const str, but not the -> &str. There are a couple ways to 'fix' this, e.g.:
- Add a ptr::str_from_utf8(*const [u8]) -> *const str function instead. This way we reuse slice ptr functions (like ptr::slice_from_raw_parts) and split safety into 2 parts: ptr validity and utf8 validity.
- Add a -> &str counterpart too, like str::from_raw_parts (similar to slice::from_raw_parts)

I'm waiting for the decision on str_from_raw_parts{_mut} to fix everything and one go.

bors · 2021-07-28T03:43:37Z

☔ The latest upstream changes (presumably #85769) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2021-08-15T08:00:04Z

Triage: Still has conflicts

bors · 2021-08-18T01:09:00Z

☔ The latest upstream changes (presumably #86808) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2021-08-25T11:17:18Z

☔ The latest upstream changes (presumably #87875) made this pull request unmergeable. Please resolve the merge conflicts.

The functions are under feature gate `str_from_raw_parts` and are similar to `slice_from_raw_parts`, `slice_from_raw_parts_mut`.

These items allow to make inherent impls for `*const str` and `*mut str`.

This patch adds the following methods to `*const str` and `*mut str`: - `len` - `as_ptr` (`as_mut_ptr`) - `get_unchecked` (`get_unchecked_mut`) Similar methods have already existed for raw slices.

This patch adds the following methods to `NonNull<str>`: - `str_from_raw_parts` - `len` - `as_non_null_ptr` - `as_mut_ptr` - `get_unchecked_mut` Similar methods have already existed for raw slices, raw strings and nonnull raw strings.

bors · 2021-11-25T02:46:46Z

☔ The latest upstream changes (presumably #91203) made this pull request unmergeable. Please resolve the merge conflicts.

WaffleLapkin · 2021-11-25T12:27:28Z

Closing in favour of #91216 and #91218

rust-highfive assigned m-ou-se May 29, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 29, 2021

joelpalmer added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2021

m-ou-se added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2021

WaffleLapkin force-pushed the str_ptr_len_get branch from 3a74406 to 7425507 Compare August 16, 2021 09:55

WaffleLapkin force-pushed the str_ptr_len_get branch from 7425507 to 55646f0 Compare August 18, 2021 12:07

Add ptr::{str_from_raw_parts, str_from_raw_parts_mut} functions

d2e78d8

The functions are under feature gate `str_from_raw_parts` and are similar to `slice_from_raw_parts`, `slice_from_raw_parts_mut`.

WaffleLapkin force-pushed the str_ptr_len_get branch from 55646f0 to 4c8427c Compare September 3, 2021 17:24

This comment has been minimized.

Sign in to view

WaffleLapkin added 3 commits September 4, 2021 17:19

Add const_str_ptr and mut_str_ptr lang items

ab96836

These items allow to make inherent impls for `*const str` and `*mut str`.

Add raw str methods

c482f97

This patch adds the following methods to `*const str` and `*mut str`: - `len` - `as_ptr` (`as_mut_ptr`) - `get_unchecked` (`get_unchecked_mut`) Similar methods have already existed for raw slices.

Add nonnull raw str methods

3ebb712

This patch adds the following methods to `NonNull<str>`: - `str_from_raw_parts` - `len` - `as_non_null_ptr` - `as_mut_ptr` - `get_unchecked_mut` Similar methods have already existed for raw slices, raw strings and nonnull raw strings.

WaffleLapkin force-pushed the str_ptr_len_get branch from 4c8427c to 3ebb712 Compare September 4, 2021 14:20

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2021

camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2021

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021

This was referenced Nov 25, 2021

Add ptr::{str_from_raw_parts, str_from_raw_parts_mut} functions #91216

Closed

Add str pointer methods (take 2) #91218

Closed

WaffleLapkin closed this Nov 25, 2021

WaffleLapkin deleted the str_ptr_len_get branch December 21, 2021 00:01

Add str pointer methods #85816

Add str pointer methods #85816

Uh oh!

Conversation

WaffleLapkin commented May 29, 2021

Uh oh!

rust-highfive commented May 29, 2021

Uh oh!

jfrimmel commented May 30, 2021

Uh oh!

WaffleLapkin commented May 30, 2021

Uh oh!

jfager commented May 30, 2021

Uh oh!

WaffleLapkin commented May 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m-ou-se commented May 31, 2021

Uh oh!

WaffleLapkin commented May 31, 2021

Uh oh!

crlf0710 commented Jul 4, 2021

Uh oh!

m-ou-se commented Jul 5, 2021

Uh oh!

dtolnay commented Jul 5, 2021

Uh oh!

WaffleLapkin commented Jul 5, 2021

Uh oh!

WaffleLapkin commented Jul 17, 2021

Uh oh!

bors commented Jul 28, 2021

Uh oh!

JohnCSimon commented Aug 15, 2021

Uh oh!

bors commented Aug 18, 2021

Uh oh!

bors commented Aug 25, 2021

Uh oh!

This comment has been minimized.

bors commented Nov 25, 2021

Uh oh!

WaffleLapkin commented Nov 25, 2021

Uh oh!

Uh oh!

WaffleLapkin commented May 30, 2021 •

edited

Loading